[XEN] Fix the x86 emulator to safely fail when it turns out the

faulting memory access was to/from an implicit memory operand
(typically either an instruction fetch or stack access).
Rationalise use of macros for page fault error code flags.
This patch supercedes the fix in changeset 11242.
Signed-off-by: Keir Fraser <keir@xensource.com>

diff --git a/tools/tests/test_x86_emulator.c b/tools/tests/test_x86_emulator.c
index 11755c3373aebdb3a2a4a37adc10771cbdaf0956..d3b7af0a7c982b2e3b50c7ecffc48ed0729cea5d 100644 (file)
--- a/tools/tests/test_x86_emulator.c
+++ b/tools/tests/test_x86_emulator.c
@@ -15,6 +15,8 @@ typedef int64_t            s64;
 #include <asm-x86/x86_emulate.h>
 #include <sys/mman.h>
 
+#define PFEC_write_access (1U<<1)
+
 static int read_any(
     unsigned long addr,
     unsigned long *val,
@@ -105,6 +107,7 @@ int main(int argc, char **argv)
     regs.eflags = 0x200;
     regs.eip    = (unsigned long)&instr[0];
     regs.ecx    = 0x12345678;
+    regs.error_code = PFEC_write_access;
     ctxt.cr2    = (unsigned long)res;
     *res        = 0x7FFFFFFF;
     rc = x86_emulate_memop(&ctxt, &emulops);
@@ -125,6 +128,7 @@ int main(int argc, char **argv)
     regs.ecx    = 0x12345678UL;
 #endif
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = 0;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x92345677) || 
@@ -139,6 +143,7 @@ int main(int argc, char **argv)
     regs.eip    = (unsigned long)&instr[0];
     regs.ecx    = ~0UL;
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = 0;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x92345677) || 
@@ -154,6 +159,7 @@ int main(int argc, char **argv)
     regs.eax    = 0x92345677UL;
     regs.ecx    = 0xAA;
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x923456AA) || 
@@ -170,6 +176,7 @@ int main(int argc, char **argv)
     regs.eax    = 0xAABBCC77UL;
     regs.ecx    = 0xFF;
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x923456AA) || 
@@ -186,6 +193,7 @@ int main(int argc, char **argv)
     regs.eip    = (unsigned long)&instr[0];
     regs.ecx    = 0x12345678;
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x12345678) || 
@@ -203,6 +211,7 @@ int main(int argc, char **argv)
     regs.eax    = 0x923456AAUL;
     regs.ecx    = 0xDDEEFF00L;
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0xDDEEFF00) || 
@@ -240,6 +249,7 @@ int main(int argc, char **argv)
     regs.eip    = (unsigned long)&instr[0];
     regs.edi    = (unsigned long)res;
     ctxt.cr2    = regs.edi;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x2233445D) ||
@@ -261,6 +271,7 @@ int main(int argc, char **argv)
     regs.eip    = (unsigned long)&instr[0];
     regs.edi    = (unsigned long)res;
     ctxt.cr2    = regs.edi;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (res[0] != 0x9999AAAA) ||
@@ -275,6 +286,7 @@ int main(int argc, char **argv)
     regs.eip    = (unsigned long)&instr[0];
     regs.edi    = (unsigned long)res;
     ctxt.cr2    = regs.edi;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (res[0] != 0x9999AAAA) ||
@@ -292,6 +304,7 @@ int main(int argc, char **argv)
     regs.ecx    = 0x12345678;
     ctxt.cr2    = (unsigned long)res;
     *res        = 0x82;
+    regs.error_code = 0;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) ||
          (*res != 0x82) ||
@@ -307,6 +320,7 @@ int main(int argc, char **argv)
     regs.ecx    = 0x12345678;
     ctxt.cr2    = (unsigned long)res;
     *res        = 0x1234aa82;
+    regs.error_code = 0;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) ||
          (*res != 0x1234aa82) ||

diff --git a/xen/arch/x86/shadow2.c b/xen/arch/x86/shadow2.c
index f5278c4a0e156f39fa70497bd6caeebc5175f459..943091b7164b3d6df42f577927656bbcf4b38109 100644 (file)
--- a/xen/arch/x86/shadow2.c
+++ b/xen/arch/x86/shadow2.c
@@ -2893,7 +2893,7 @@ static int sh2_page_fault(struct vcpu *v,
     // i.e. ring 3.  Such errors are not caused or dealt with by the shadow
     // code.
     //
-    if ( (regs->error_code & X86_PFEC_SUPERVISOR_FAULT) &&
+    if ( (regs->error_code & PFEC_user_mode) &&
          !(accumulated_gflags & _PAGE_USER) )
     {
         /* illegal user-mode access to supervisor-only page */
@@ -2903,7 +2903,7 @@ static int sh2_page_fault(struct vcpu *v,
 
     // Was it a write fault?
     //
-    if ( regs->error_code & X86_PFEC_WRITE_FAULT )
+    if ( regs->error_code & PFEC_write_access )
     {
         if ( unlikely(!(accumulated_gflags & _PAGE_RW)) )
         {
@@ -2917,7 +2917,7 @@ static int sh2_page_fault(struct vcpu *v,
         // marked "do not execute".  Such errors are not caused or dealt with
         // by the shadow code.
         //
-        if ( regs->error_code & X86_PFEC_INSN_FETCH_FAULT )
+        if ( regs->error_code & PFEC_insn_fetch )
         {
             if ( accumulated_gflags & _PAGE_NX_BIT )
             {
@@ -2960,8 +2960,8 @@ static int sh2_page_fault(struct vcpu *v,
      * for the shadow entry, since we might promote a page here. */
     // XXX -- this code will need to change somewhat if/when the shadow code
     // can directly map superpages...
-    ft = ((regs->error_code & X86_PFEC_WRITE_FAULT) 
-          ? ft_demand_write : ft_demand_read);
+    ft = ((regs->error_code & PFEC_write_access) ?
+          ft_demand_write : ft_demand_read);
     ptr_sl1e = shadow_get_and_create_l1e(v, &gw, &sl1mfn, ft);
     ASSERT(ptr_sl1e);
 

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index bc756699ee1e2705440623b117c8615837cabd28..f47a6822970cdeba2ed4520aae4ac23596ab9d70 100644 (file)
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -686,9 +686,9 @@ void propagate_page_fault(unsigned long addr, u16 error_code)
     v->vcpu_info->arch.cr2           = addr;
 
     /* Re-set error_code.user flag appropriately for the guest. */
-    error_code &= ~PGERR_user_mode;
+    error_code &= ~PFEC_user_mode;
     if ( !guest_kernel_mode(v, guest_cpu_user_regs()) )
-        error_code |= PGERR_user_mode;
+        error_code |= PFEC_user_mode;
 
     ti = &v->arch.guest_context.trap_ctxt[TRAP_page_fault];
     tb->flags = TBF_EXCEPTION | TBF_EXCEPTION_ERRCODE;
@@ -768,17 +768,17 @@ static int __spurious_page_fault(
     unsigned int required_flags, disallowed_flags;
 
     /* Reserved bit violations are never spurious faults. */
-    if ( regs->error_code & PGERR_reserved_bit )
+    if ( regs->error_code & PFEC_reserved_bit )
         return 0;
 
     required_flags  = _PAGE_PRESENT;
-    if ( regs->error_code & PGERR_write_access )
+    if ( regs->error_code & PFEC_write_access )
         required_flags |= _PAGE_RW;
-    if ( regs->error_code & PGERR_user_mode )
+    if ( regs->error_code & PFEC_user_mode )
         required_flags |= _PAGE_USER;
 
     disallowed_flags = 0;
-    if ( regs->error_code & PGERR_instr_fetch )
+    if ( regs->error_code & PFEC_insn_fetch )
         disallowed_flags |= _PAGE_NX;
 
     mfn = cr3 >> PAGE_SHIFT;
@@ -886,7 +886,7 @@ static int fixup_page_fault(unsigned long addr, struct cpu_user_regs *regs)
          guest_kernel_mode(v, regs) &&
          /* Do not check if access-protection fault since the page may 
             legitimately be not present in shadow page tables */
-         ((regs->error_code & PGERR_write_access) == PGERR_write_access) &&
+         ((regs->error_code & PFEC_write_access) == PFEC_write_access) &&
          ptwr_do_page_fault(d, addr, regs) )
         return EXCRET_fault_fixed;
 
@@ -1100,7 +1100,7 @@ static int emulate_privileged_op(struct cpu_user_regs *regs)
             if ( (rc = copy_to_user((void *)regs->edi, &data, op_bytes)) != 0 )
             {
                 propagate_page_fault(regs->edi + op_bytes - rc,
-                                     PGERR_write_access);
+                                     PFEC_write_access);
                 return EXCRET_fault_fixed;
             }
             regs->edi += (int)((regs->eflags & EF_DF) ? -op_bytes : op_bytes);

diff --git a/xen/arch/x86/x86_emulate.c b/xen/arch/x86/x86_emulate.c
index 4016aa77e33a678588bee5452eb9c9533aeb4275..e7f1a77196c4fb479418ad1432ea94eeb7492342 100644 (file)
--- a/xen/arch/x86/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate.c
@@ -21,6 +21,11 @@
 #endif
 #include <asm-x86/x86_emulate.h>
 
+#ifndef PFEC_write_access
+#define PFEC_write_access (1U<<1)
+#define PFEC_insn_fetch   (1U<<4)
+#endif
+
 /*
  * Opcode effective-address decode tables.
  * Note that we only emulate instructions that have at least one memory
@@ -444,6 +449,13 @@ x86_emulate_memop(
     /* Shadow copy of register state. Committed on successful emulation. */
     struct cpu_user_regs _regs = *ctxt->regs;
 
+    /*
+     * We do not emulate faults on instruction fetch. We assume that the
+     * guest never executes out of a special memory area.
+     */
+    if ( _regs.error_code & PFEC_insn_fetch )
+        return -1;
+
     switch ( mode )
     {
     case X86EMUL_MODE_REAL:
@@ -620,6 +632,14 @@ x86_emulate_memop(
         }
         break;
     case DstMem:
+        /*
+         * We expect that the fault occurred while accessing the explicit
+         * destination memory operand. This is clearly not the case if the
+         * fault occurred on a read access (eg. POP has an *implicit* operand
+         * but we expect that the guest never uses special memory as stack).
+         */
+        if ( !(_regs.error_code & PFEC_write_access) )
+            goto cannot_emulate;
         dst.type  = OP_MEM;
         dst.ptr   = (unsigned long *)cr2;
         dst.bytes = (d & ByteOp) ? 1 : op_bytes;
@@ -664,6 +684,14 @@ x86_emulate_memop(
     case SrcMem:
         src.bytes = (d & ByteOp) ? 1 : op_bytes;
     srcmem_common:
+        /*
+         * We expect that the fault occurred while accessing the explicit
+         * source memory operand. This is clearly not the case if the fault
+         * occurred on a write access (eg. PUSH has an *implicit* operand
+         * but we expect that the guest never uses special memory as stack).
+         */
+        if ( _regs.error_code & PFEC_write_access )
+            goto cannot_emulate;
         src.type  = OP_MEM;
         src.ptr   = (unsigned long *)cr2;
         if ( (rc = ops->read_emulated((unsigned long)src.ptr, 
@@ -846,9 +874,6 @@ x86_emulate_memop(
             emulate_1op("dec", dst, _regs.eflags);
             break;
         case 6: /* push */
-            /* Don't emulate if fault was on stack */
-            if ( _regs.error_code & 2 )
-                goto cannot_emulate; 
             /* 64-bit mode: PUSH always pushes a 64-bit operand. */
             if ( mode == X86EMUL_MODE_PROT64 )
             {
@@ -923,7 +948,7 @@ x86_emulate_memop(
     case 0xa4 ... 0xa5: /* movs */
         dst.type  = OP_MEM;
         dst.bytes = (d & ByteOp) ? 1 : op_bytes;
-        if ( _regs.error_code & 2 )
+        if ( _regs.error_code & PFEC_write_access )
         {
             /* Write fault: destination is special memory. */
             dst.ptr = (unsigned long *)cr2;
@@ -1165,7 +1190,7 @@ x86_emulate_write_std(
 
     if ( (rc = copy_to_user((void *)addr, (void *)&val, bytes)) != 0 )
     {
-        propagate_page_fault(addr + bytes - rc, PGERR_write_access);
+        propagate_page_fault(addr + bytes - rc, PFEC_write_access);
         return X86EMUL_PROPAGATE_FAULT;
     }
 

diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 81c8757f8e5aff8a7a587d551870cad5571ea755..196a5bc5bcc5ab341b103dd21f80445423fac888 100644 (file)
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -130,11 +130,11 @@
 #define TF_kernel_mode         (1<<_TF_kernel_mode)
 
 /* #PF error code values. */
-#define PGERR_page_present   (1U<<0)
-#define PGERR_write_access   (1U<<1)
-#define PGERR_user_mode      (1U<<2)
-#define PGERR_reserved_bit   (1U<<3)
-#define PGERR_instr_fetch    (1U<<4)
+#define PFEC_page_present   (1U<<0)
+#define PFEC_write_access   (1U<<1)
+#define PFEC_user_mode      (1U<<2)
+#define PFEC_reserved_bit   (1U<<3)
+#define PFEC_insn_fetch     (1U<<4)
 
 #ifndef __ASSEMBLY__
 

diff --git a/xen/include/asm-x86/shadow2-types.h b/xen/include/asm-x86/shadow2-types.h
index fcc6b466f8ba7ec7f3c829542c55aad9534b75d0..13107f4566196a43f0be4a2ec5748f67c7194d3c 100644 (file)
--- a/xen/include/asm-x86/shadow2-types.h
+++ b/xen/include/asm-x86/shadow2-types.h
@@ -461,19 +461,6 @@ struct shadow2_walk_t
     mfn_t l1mfn;                /* MFN that the level 1 entry is in */
 };
 
-
-/* X86 error code bits:
- * These bits certainly ought to be defined somewhere other than here,
- * but until that place is determined, here they sit.
- *
- * "PFEC" == "Page Fault Error Code"
- */
-#define X86_PFEC_PRESENT            1  /* 0 == page was not present */
-#define X86_PFEC_WRITE_FAULT        2  /* 0 == reading, 1 == writing */
-#define X86_PFEC_SUPERVISOR_FAULT   4  /* 0 == supervisor-mode, 1 == user */
-#define X86_PFEC_RESERVED_BIT_FAULT 8  /* 1 == reserved bits set in pte */
-#define X86_PFEC_INSN_FETCH_FAULT  16  /* 0 == normal, 1 == instr'n fetch */
-
 /* macros for dealing with the naming of the internal function names of the
  * shadow code's external entry points.
  */